Skip to content

[HDX-5080] Chart editor UI for metric formulas - #2909

Open
wrn14897 wants to merge 2 commits into
mainfrom
warren/HDX-5080-render-metric-formula-UI
Open

[HDX-5080] Chart editor UI for metric formulas#2909
wrn14897 wants to merge 2 commits into
mainfrom
warren/HDX-5080-render-metric-formula-UI

Conversation

@wrn14897

@wrn14897 wrn14897 commented Aug 14, 2026

Copy link
Copy Markdown
Member

Summary

Exposes metric formulas (HDX-5078's formulas config, rendered by HDX-5079) in the chart editor for metric sources, so a derived series like A / (A + B + C) * 100 can be built, validated, saved, and reloaded from the UI.

Rebased onto main now that #2908 (HDX-5079 rendering) has merged.

Editor

  • Formula rows (ChartFormulaEditor) on metric-source builder charts (time series / table / number): "Add Formula" appends a row with a monospace letter-ref expression input, an alias, a per-formula number format (reuses the per-series format drawer), and "Remove Formula".
  • Inline validation with the structured validator from HDX-5078 (validateFormula): malformed expressions, unknown series refs, constant-only expressions, etc. surface live under the input; validateChartForm blocks save/run with the same messages so an invalid expression can never reach ClickHouse.
  • Letter badges (A, B, C, ...) on metric series rows so formula refs are discoverable.
  • "Show input series" toggle drives showOperandSeries (formula + raw operand series vs formula column(s) only). Adding a formula on a Number tile defaults operands to hidden, since Number tiles render the first value column.
  • Mutual exclusion with ratio: the "As Ratio" switch is hidden while a formula exists, and "Add Formula" is hidden while ratio mode is on (formulas supersede ratio in the renderer).
  • The Number-tile series cap (1, or 2 for ratio) is lifted when formulas exist, so operand-only series like A / (A + B + C) can be built.
  • normalizeChartConfig strips formulas/showOperandSeries on save for non-metric sources and for display types the composed metric query does not render (pie/bar/heatmap/search/patterns), mirroring the existing metricName/having stripping. The form state keeps them, so switching back restores the rows.

Rendering consumers (positional value-column contract)

The composed metric query projects operand columns (unless hidden) then formula columns, ahead of group-by passthrough columns. Updated the consumers that map columns positionally:

  • useChartNumberFormats: operand columns → select[i].numberFormat, formula columns → formulas[j].numberFormat, both falling back to the chart-wide format; chart-wide axis format prefers formula formats when operands are hidden.
  • New getBuilderValueColumnCount helper (formula/ratio-aware) used by DBTableChart for group-by column inference; per-column color mapping skips hidden-operand formula configs.
  • DBTimeChart drill-down skips the value-range filter when operands are hidden (formula columns don't map onto select expressions).
  • Legend/tooltip naming needs no changes — formula columns arrive as named result columns (alias || expression).

Persistence needs no API changes: tiles validate against SavedChartConfigSchema, which already carries formulas/showOperandSeries, and builderToRawSql already rejects formula configs with a clear message on the Builder → SQL switch.

Alerts on formula tiles (packages/api)

Contrary to #2908's "alerts work with no changes" claim, the alert task does not run the tile config as-is — getChartConfigFromAlert rebuilds it from an explicit field list that dropped formulas/showOperandSeries. An alert on a formula tile therefore queried only the raw operand series and compared the threshold against the last operand's value (e.g. 740442112.0 meets or exceeds 0.1 for a byte-valued operand), regardless of the tile's "Show input series" toggle. Fixed here:

  • formulas is passed through, and operand columns are always dropped from the alert query (showOperandSeries: false) so the formula is the value column parseAlertData picks — the alert evaluates exactly what the tile displays.
  • Drive-by with the same omission shape: ratioMode is now passed through, so grouped share_of_total ratio tile alerts no longer silently evaluate as per_group.
  • New integration tests (make dev-int FILE=checkAlerts, 168 passing): formula value drives the alert (fixture chosen so the formula result differs from both operands), toggle-independence, NULL formula (zero denominator) skipped without NaN history, and share_of_total honored (asserting the exact share value a per_group fallback couldn't produce).

Testing

  • make ci-lint, make ci-unit pass.
  • New unit tests:
    • DBEditTimeChartForm.test.tsx: Add/Remove Formula, inline validation (malformed / unknown ref / clears when fixed), save round-trip, save blocked on invalid expression, ratio mutual exclusion, showOperandSeries toggle, non-metric sources show no formula controls.
    • ChartEditor/utils.test.ts: validateChartForm formula rules (including the Number-tile cap lift) and normalization stripping/round-trip.
    • source.test.ts: useChartNumberFormats formula-column mapping (operands shown/hidden, ratio precedence, chart-format fallbacks) and getBuilderValueColumnCount.
  • New dashboard E2E (make dev-e2e FILE=dashboard GREP="Metric formulas", passing): creates a metric table tile with two gauge series + A / (A + B) * 100, asserts the inline error for an invalid ref, hides operands, saves, reloads the page, verifies the formula column renders with a finite value, and reopens the editor to verify the round-trip.

How to test on Vercel preview

  1. Open a dashboard → Add tile → select a metrics source.
  2. Add two series (note the A/B badges), click Add Formula, enter A / (A + B) * 100.
  3. Try A / C to see the inline error; toggle Show input series; save, reload, and confirm the tile renders the formula series.

References

Screenshots

image

@changeset-bot

changeset-bot Bot commented Aug 14, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 43cd185

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@hyperdx/api Minor
@hyperdx/app Minor
@hyperdx/otel-collector Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel

vercel Bot commented Aug 14, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview Aug 19, 2026 1:52am
hyperdx-storybook Ready Ready Preview Aug 19, 2026 1:52am

Request Review

@greptile-apps

greptile-apps Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR exposes metric formulas in the chart editor and coordinates their persistence, rendering metadata, and alert evaluation.

  • Adds formula editing, inline validation, aliases, formats, operand visibility, and series-reference badges.
  • Updates table/time-chart consumers for formula value-column layouts.
  • Preserves formulas and ratio modes when rebuilding metric tile configurations for alerts.
  • Adds unit, integration, and end-to-end coverage for formula authoring, rendering, and alerts.

Confidence Score: 4/5

The PR is not yet safe to merge because series edits can still silently change which operands existing formulas evaluate.

Moving, duplicating, or removing a series mutates its positional index without rewriting formula expressions, while validation checks only whether letter references remain within the current series count; an expression can therefore stay valid but compute against different metrics.

Files Needing Attention: packages/app/src/components/DBEditTimeChartForm/ChartSeriesEditor.tsx, packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx

Important Files Changed

Filename Overview
packages/app/src/components/DBEditTimeChartForm/ChartFormulaEditor.tsx Adds formula expression, alias, formatting, removal, and live validation controls.
packages/app/src/components/DBEditTimeChartForm/ChartSeriesEditor.tsx Adds positional reference badges, but existing series mutations still leave formula expressions unchanged.
packages/app/src/components/ChartEditor/utils.ts Adds formula-aware normalization and validation for supported metric chart types.
packages/app/src/source.ts Maps number formats and value-column counts across operand, formula, and ratio query layouts.
packages/app/src/components/DBTableChart.tsx Uses formula-aware value-column counts for group-by inference and column coloring.
packages/app/src/components/DBTimeChart.tsx Avoids invalid value-range drill-down filtering when formula operands are hidden.
packages/api/src/tasks/checkAlerts/index.ts Preserves formulas and ratio mode in tile alerts while forcing formula-only alert query results.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Editor[Chart editor series and formulas] --> Validate[Formula validation]
  Validate --> Config[Saved metric chart config]
  Config --> Query[Composed metric query]
  Query --> Operands[Operand columns]
  Query --> Formulas[Formula columns]
  Operands --> Charts[Chart consumers]
  Formulas --> Charts
  Config --> AlertConfig[Alert config rebuild]
  AlertConfig --> FormulaOnly[Formula-only alert query]
  FormulaOnly --> Threshold[Threshold evaluation]
Loading

Reviews (6): Last reviewed commit: "fix: evaluate metric formulas in tile al..." | Re-trigger Greptile

@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Deep Review

Scope: PR #2909 — Chart editor UI for metric formulas (HDX-5080), base a94d6da8. Adds formulas/showOperandSeries to the metric builder chart editor, updates positional value-column consumers (source.ts, DBTableChart, DBTimeChart), and changes the tile-alert task to evaluate the formula column.

The alert value-column mechanics were verified directly against source: parseAlertData selects the last column present in meta.valueColumnNames, and getResponseMetadata builds that set from every numeric-typed column. The renderer projects operand columns (unless hidden) then formula columns, ahead of group-by passthrough — confirming the diff's positional contract holds for the common case (single formula, non-numeric group-by).

🟡 P2 -- recommended

  • packages/api/src/tasks/checkAlerts/index.ts:721 -- Passing ratioMode through changes the evaluated value for already-saved grouped share_of_total ratio-tile alerts, which previously dropped the field and evaluated as per_group; existing alerts can fire or resolve differently on the first post-deploy evaluation with no migration or operator notice.
    • Fix: Confirm the semantics change is intended and call it out in release notes so operators re-baseline affected ratio alerts.
  • packages/app/src/source.ts:664 -- useChartNumberFormats reimplements the operand-then-formula projection ordering inline rather than reusing the new getBuilderValueColumnCount, so the same positional contract now lives in four places (renderer, getBuilderValueColumnCount, useChartNumberFormats, DBTimeChart/DBTableChart) and a future projection change can silently desync formatting from column positions.
    • Fix: Derive column ordering/count from one shared helper and have the formatter and table/drill-down consumers read from it.
🔵 P3 nitpicks (2)
  • packages/app/src/components/DBEditTimeChartForm/ChartEditorControls.tsx:120 -- The showOperandSeries tri-state is implicit: undefined means "shown", false means "hidden", and true is never written, so readers must infer the mapping from scattered === false checks.
    • Fix: Document the tri-state at the field/type definition, or normalize to an explicit boolean on save.
  • packages/app/src/components/DBEditTimeChartForm/ChartFormulaEditor.tsx:1 -- The new formula row fires onSubmit() on blur, Enter, alias change, and format change; consider whether the resulting query re-runs are debounced consistently with the existing series editor to avoid redundant ClickHouse round-trips while editing.
    • Fix: Verify the submit cadence matches ChartSeriesEditor and debounce if it does not.

Reviewers (2 returned + orchestrator verification): ce-agent-native-reviewer, ce-learnings-researcher, plus direct orchestrator code verification of the alert-evaluation and renderer-projection paths. Nine dispatched persona reviewers (correctness, adversarial, testing, maintainability, project-standards, kieran-typescript, julik-frontend-races, api-contract, reliability) had not returned when output was finalized; findings above reflect only confirmed, diff-grounded issues and should not be read as a clean bill from those lenses.

Testing gaps:

  • No test for a formula-tile alert with a numeric group-by column: getResponseMetadata would add that group column to valueColumnNames, and parseAlertData's "last value column wins" would select it over the formula (a pre-existing grouped-alert pattern, now reachable for formula tiles).
  • No test asserting which formula drives the alert when a tile carries multiple formulas.
  • No test for an API/agent-authored config with formulas plus a non-composed displayType (e.g. Pie), which bypasses the UI-only normalizeChartConfig stripping.

@wrn14897
wrn14897 force-pushed the warren/HDX-5079-render-formulas branch from 8d9e37e to 41b2354 Compare August 17, 2026 19:03
@wrn14897
wrn14897 force-pushed the warren/HDX-5080-render-metric-formula-UI branch from 2067484 to 226e1a9 Compare August 17, 2026 23:08
@wrn14897
wrn14897 changed the base branch from warren/HDX-5079-render-formulas to main August 17, 2026 23:09
@wrn14897
wrn14897 force-pushed the warren/HDX-5080-render-metric-formula-UI branch from 226e1a9 to 1010ee4 Compare August 17, 2026 23:23
@github-actions github-actions Bot added the review/tier-3 Standard — full human review required label Aug 17, 2026
@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

🟡 Tier 3 — Standard

Introduces new logic, modifies core functionality, or touches areas with non-trivial risk.

Why this tier:

  • Diff size: 508 production lines changed (Tier 2 max: < 250)
  • Cross-layer change: touches frontend (packages/app) + backend (packages/api)

Additional context: touches background tasks or the delivery pipeline lightly (11 lines, under the 30-line bar for Tier 4)

Review process: Full human review — logic, architecture, edge cases.
SLA: First-pass feedback within 1 business day.

Stats
  • Production files changed: 8
  • Production lines changed: 508 (+ 1057 in test files, excluded from tier calculation)
  • Branch: warren/HDX-5080-render-metric-formula-UI
  • Author: wrn14897

To override this classification, remove the review/tier-3 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

E2E Test Results

All tests passed • 302 passed • 1 skipped • 975s

Status Count
✅ Passed 302
❌ Failed 0
⚠️ Flaky 1
⏭️ Skipped 1

Tests ran across 4 shards in parallel.

View full report →

@pulpdrew

pulpdrew commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

One thing I noticed while testing: HAVING is applied within each CTE instead of to the final join, which is (to me at least) unexpected/unclear, and makes it hard to know what aliases/series/formulas I can reference.

Same with order by - I would expect to be able to reference any of the columns which are being outputted in the table in ORDER BY or HAVING.

Screenshot 2026-08-18 at 9 05 06 AM

@pulpdrew

pulpdrew commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Also, for number charts I suspect we need to make sure that (a) only one formula is allowed (b) show input series is hardcoded to false and (c) the chart renders the formula column and not any series columns.

I see we do (b) already when adding a formula, but not when a formula already exists and the display type is changed to number

Screenshot 2026-08-18 at 9 10 25 AM

Expose metric formulas (HDX-5078 schema, HDX-5079 rendering) in the chart
editor for metric sources:

- Formula rows in the builder (Add Formula / Remove Formula) with a
  letter-ref expression input, alias, and per-formula number format
- Inline validation via the structured validator (malformed expressions,
  unknown series refs) plus save-time validation in validateChartForm
- Letter badges (A, B, C) on metric series rows so refs are discoverable
- Show input series toggle (showOperandSeries) for formula-only output;
  Number tiles default to hidden operands since they render the first
  value column
- Formulas and the As Ratio toggle are mutually exclusive in the editor
- normalizeChartConfig strips formulas for non-metric sources and
  display types the composed metric query does not render
- useChartNumberFormats / DBTableChart / DBTimeChart updated for the
  formula-aware value-column projection (operands first unless hidden,
  then formula columns)
getChartConfigFromAlert rebuilds a builder tile's chart config from an
explicit field list and dropped formulas/showOperandSeries, so an alert
on a formula tile queried only the raw operand series and compared the
threshold against the last operand's value (e.g. raw bytes) instead of
the formula result — regardless of the tile's "Show input series"
toggle.

- Pass formulas through and always drop operand columns from the alert
  query (showOperandSeries: false) so the formula is the value column
  parseAlertData picks
- Also pass ratioMode through: grouped share_of_total ratio tile alerts
  previously evaluated as per_group (the default)
- Integration tests: formula value drives the alert (value distinct from
  both operands), toggle-independence, NULL formula (zero denominator)
  skipped, and share_of_total ratioMode honored
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review/tier-3 Standard — full human review required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants